Skip to content

perf(region): memoize rebuildBlockLedger per snapshot to cut O(B^2*N) rebuilds (#109) - #123

Closed
ranxianglei wants to merge 1 commit into
Tyan66666:mainfrom
ranxianglei:2026-09-06_issue109-ledger-caching
Closed

perf(region): memoize rebuildBlockLedger per snapshot to cut O(B^2*N) rebuilds (#109)#123
ranxianglei wants to merge 1 commit into
Tyan66666:mainfrom
ranxianglei:2026-09-06_issue109-ledger-caching

Conversation

@ranxianglei

Copy link
Copy Markdown
Contributor

What

Fixes #109rebuildBlockLedger was being recomputed many times inside hot loops instead of once per tool call.

Root cause

rebuildBlockLedger scans the full durable log and calls summarySeqOfCompaction (itself O(N)) once per compaction block → O(B·N) per call. Several helpers wrap it (blockRegistry, expandShadowedSeqs, blockRefForSummarySeq, compactionIdsOfKernelBlocks, summarySeqOfKernelBlock) and are invoked per element in loops:

  • search_contextbuildSearchDocs loops expandShadowedSeqs over every block → O(B²·N)
  • compresshandleCompress calls blockRefForSummarySeq×2 + compactionIdsOfKernelBlocks per range → O(R·B·N)
  • tier-nudge → summarySeqOfKernelBlock per target block

So search / compress / nudge latency grew super-linearly with the number of blocks.

Fix

Memoize rebuildBlockLedger on the input snapshot array via a module-level WeakMap, guarded by array length. Safe because:

  • within one tool invocation every caller passes the same stable snapshot (sessionEventsOf reuses it until the next append);
  • the log is append-only, so same reference + same length ⇒ identical content;
  • after an append the snapshot is either a new array (copy-on-write) or grows in length → the key misses → recompute.

The function is pure and no caller mutates the returned ledger, so sharing the cached array is correct. Net effect: each tool call rebuilds the ledger once (O(B·N)) instead of per element — a ~B× reduction on the hot paths. Zero signature changes; all callers benefit automatically.

Verification

  • npm run typecheck
  • npm test209/209 pass (added an idempotency regression test asserting repeated calls on one snapshot return an identical ledger)
  • npm run build ✅ — dist/ regenerated for CI's dist-drift check; dist/region.d.ts is byte-identical to main (no public type change)

Files

  • src/region.ts (+7): the WeakMap memo + length guard
  • tests/region.test.ts (+21): idempotency regression test
  • dist/index.js, dist/index.js.map: rebuilt bundle

@ranxianglei

Copy link
Copy Markdown
Contributor Author

我来协助看一下这个问题,请分析并回复处理结果。

@Tyan66666

Copy link
Copy Markdown
Owner

Superseded by #134 — the same memoization already landed on main.

Evidence: main (db8be1d) carries the identical cache in src/region.ts:507:

const blockLedgerCache = new WeakMap<readonly SessionEvent[], { len: number; ledger: AcpBlockLedgerEntry[] }>()

with the same length-based invalidation at src/region.ts:510-511, and git log -S "blockLedgerCache" -- src/region.ts points at commit fceeeef (#134). The new test's intent is likewise covered by tests/search-perf.test.ts:20 ("rebuildBlockLedger is memoized per snapshot (issues #109/#133)"). This PR's patch no longer applies to main precisely because the code is already there (git apply --check fails at src/region.ts:473).

Closing as superseded. Thanks for the fix — it is in.

@Tyan66666 Tyan66666 closed this Sep 11, 2026
@Tyan66666 Tyan66666 mentioned this pull request Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[perf] rebuildBlockLedger/blockRegistry 未缓存,search/compress/nudge 内被循环 O(N) 重扫

2 participants